fix(review): require signed workspace authority on current main - #178

apps/review-service/src/main.ts (2)

52-55: 🩺 Stability & Availability | 🔵 Trivial

부팅 시점에 REVIEW_GATEWAY_CONTEXT_SECRET 검증을 추가하는 것을 권장합니다.

현재는 요청마다 secret을 읽고, 미설정 시 요청 단위로 503을 반환합니다. 동작은 fail-closed입니다. 다만 잘못 배포된 인스턴스는 트래픽을 받은 뒤에야 문제를 드러냅니다. 부팅 시 secret 존재와 최소 길이를 검사하고 readiness 프로브에 반영하면 배포 단계에서 즉시 감지할 수 있습니다.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/main.ts` around lines 52 - 55, Validate
REVIEW_GATEWAY_CONTEXT_SECRET during application startup, including its presence
and minimum required length, before serving requests. Integrate this validation
with the readiness probe so an invalid configuration keeps the instance unready
and exposes the deployment error immediately, while preserving the existing
requireTrustedWorkspaceContext request validation.

---

47-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

네 개 라우트의 컨텍스트 검증 블록이 동일하게 반복됩니다.

각 라우트가 헤더 데코레이터 3개와 requireTrustedWorkspaceContext 호출을 그대로 복제합니다. NestJS 커스텀 파라미터 데코레이터 또는 가드로 추출하면 중복이 사라지고, 새 라우트에서 검증 누락이 발생할 위험도 줄어듭니다.

주의: apps/review-service/src/review-controller-authority.test.ts의 Line 126-144는 main.ts 소스 문자열에서 데코레이터와 호출이 각각 4회 나타나는지 검사합니다. 추출 리팩터링을 진행하면 해당 테스트도 함께 갱신해야 합니다.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/main.ts` around lines 47 - 109, Extract the repeated
workspace-context header handling and requireTrustedWorkspaceContext call from
the four route methods in the review controller into a shared NestJS custom
parameter decorator or guard, then update each route to consume the trusted
workspace ID through that abstraction while preserving existing validation and
error behavior. Update review-controller-authority.test.ts to assert the
refactored source structure instead of expecting four duplicated decorator and
call occurrences.

apps/review-service/src/http-boundary.ts (1)

66-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

서명 payload가 정규화된 workspace ID를 사용한다는 점을 문서화하십시오.

workspaceContextDigest는 requireReviewWorkspaceId가 소문자로 정규화한 값을 서명 대상으로 사용합니다. 헤더 원본 값은 사용하지 않습니다. 게이트웨이가 헤더 원본 문자열로 서명하면 대소문자가 다를 때 검증이 실패합니다. 이 계약을 docstring에 명시하십시오.

📝 제안 diff

-/** Computes the SHA-256 HMAC over the canonical `life-os.workspace.v1` workspace-and-time payload. */
+/**
+ * Computes the SHA-256 HMAC over the canonical `life-os.workspace.v1` payload.
+ * The workspace ID must already be normalized to lowercase UUIDv4 form,
+ * so the gateway must sign the normalized identifier, not the raw header value.
+ */
 function workspaceContextDigest(

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/http-boundary.ts` around lines 66 - 75, Update the
docstring for workspaceContextDigest to state that the signed payload uses the
lowercase-normalized workspace ID returned by requireReviewWorkspaceId, not the
original header value, and that signing must use this normalized value
consistently for verification.

apps/review-service/src/review-controller-authority.test.ts (1)

126-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

소스 문자열 단언은 리팩터링에 취약합니다.

Line 129-140은 main.ts의 텍스트에서 데코레이터와 호출 횟수를 셉니다. 이 단언은 동작이 아니라 소스 형태를 검증합니다. 라우트 검증을 가드나 커스텀 파라미터 데코레이터로 추출하면 동작이 그대로여도 테스트가 실패합니다.

Line 146-204의 동작 기반 테스트는 이미 네 라우트 모두에서 검증 통과와 실패를 증명합니다. Line 127-128의 회귀 방지 단언(x-workspace-id, requireWorkspaceHeader 미존재)만 남기고 횟수 단언은 제거하는 방안을 검토하십시오.

테스트가 모의 호출 횟수가 아니라 실제 도메인 결과와 실패 동작을 증명해야 한다는 코딩 가이드라인에 따릅니다.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/review-controller-authority.test.ts` around lines 126
- 144, In the test case “rejects browser-selectable workspace authority on every
review route,” remove the source-text assertions that count decorator
occurrences, requireTrustedWorkspaceContext calls, and
REVIEW_GATEWAY_CONTEXT_SECRET references. Preserve only the regression
assertions rejecting x-workspace-id and requireWorkspaceHeader, relying on the
existing behavior-based tests for all four routes.

Source: Coding guidelines

apps/review-service/src/main.test.ts (1)

14-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

테스트 secret 생성 방식을 다른 테스트와 통일하는 것을 고려하십시오.

apps/review-service/src/http-boundary.test.ts와 apps/review-service/src/review-controller-authority.test.ts는 randomBytes(32).toString('base64url')을 사용합니다. 이 파일만 고정 문자열을 조합합니다. 동일한 방식을 사용하면 최소 길이 요건 충족이 명확해집니다.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/main.test.ts` around lines 14 - 21, Update the test
secret setup around GATEWAY_SECRET in main.test.ts to generate the value using
randomBytes(32).toString('base64url'), matching http-boundary.test.ts and
review-controller-authority.test.ts, and remove the fixed string composition
while preserving previousGatewaySecret handling.

apps/review-service/src/http-boundary.test.ts (2)

123-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

실패 단언을 expect(...).toThrow 형태로 바꾸는 것을 고려하십시오.

현재 sentinel throw new Error('expected trusted context rejection')은 같은 try 블록 안에 있습니다. 검증이 통과하면 이 Error가 아래 catch로 잡히고 toBeInstanceOf(HttpException) 단언이 실패합니다. 결과는 올바르지만 실패 메시지가 원인을 정확히 보여주지 않습니다.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/http-boundary.test.ts` around lines 123 - 134, Update
the stale/future/forged/unverifiable context test around
requireTrustedWorkspaceContext to use an expect(...).toThrow-style assertion
instead of the sentinel throw and surrounding try/catch. Preserve validation of
the thrown HttpException through response(error) so the expected status and code
remain asserted with an accurate failure message.

---

82-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

실패 케이스 표에 헤더 누락과 서명 형식 오류를 추가하십시오.

현재 표는 만료, 미래, 위조, 짧은 secret만 다룹니다. requireTrustedWorkspaceContext의 다음 분기는 이 파일에서 실행되지 않습니다.

• typeof headers.workspaceId !== 'string' 등 헤더 누락 분기
• !BASE64URL_SHA256_PATTERN.test(headers.signature) 서명 길이·문자 오류 분기
• !UNIX_SECONDS_PATTERN.test(headers.issuedAt) 비숫자 issuedAt 분기
• !Number.isSafeInteger(nowSeconds) || nowSeconds < 0 분기

또한 issuedAtSeconds의 !Number.isSafeInteger(...) 검사는 UNIX_SECONDS_PATTERN이 최대 13자리를 허용하므로 도달할 수 없습니다. 이 분기는 100% 분기 커버리지 게이트를 충족할 수 없습니다. 해당 검사를 제거하거나, 패턴을 넓혀 도달 가능하게 하십시오.

커버리지 게이트를 적용하는 패키지는 100% statement/branch/function/line 커버리지를 유지해야 한다는 코딩 가이드라인에 따릅니다.

🧪 표에 추가할 케이스 예시

     {
       headers: {
         workspaceId: WORKSPACE_ID,
         issuedAt: String(NOW_SECONDS),
         signature: signature(String(NOW_SECONDS)),
       },
       secret: 'too-short',
       status: 503,
       code: 'gateway_context_unavailable',
     },
+    {
+      headers: {
+        workspaceId: undefined,
+        issuedAt: String(NOW_SECONDS),
+        signature: signature(String(NOW_SECONDS)),
+      },
+      secret: SECRET,
+      status: 401,
+      code: 'invalid_gateway_context',
+    },
+    {
+      headers: {
+        workspaceId: WORKSPACE_ID,
+        issuedAt: 'not-a-timestamp',
+        signature: signature(String(NOW_SECONDS)),
+      },
+      secret: SECRET,
+      status: 401,
+      code: 'invalid_gateway_context',
+    },
+    {
+      headers: {
+        workspaceId: WORKSPACE_ID,
+        issuedAt: String(NOW_SECONDS),
+        signature: 'A'.repeat(42),
+      },
+      secret: SECRET,
+      status: 401,
+      code: 'invalid_gateway_context',
+    },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/review-service/src/http-boundary.test.ts` around lines 82 - 134, Update
the failure-case table exercising requireTrustedWorkspaceContext to cover
missing or non-string workspaceId, malformed signature characters/length,
nonnumeric issuedAt, and invalid nowSeconds values, while asserting the
appropriate rejection responses. Also make the issuedAtSeconds safe-integer
branch reachable by widening UNIX_SECONDS_PATTERN, or remove that redundant
check if the existing pattern is intentionally bounded; preserve 100% statement,
branch, function, and line coverage.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes